-
Notifications
You must be signed in to change notification settings - Fork 133
fix: add safety fallback for EMBEDDING_MAX_BATCH_SIZE parsing #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds guarded parsing for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/config.py`:
- Around line 15-19: The except handler for parsing EMBEDDING_MAX_BATCH_SIZE
references the logging module but logging is not imported, which will raise
NameError; fix by adding an import for the logging module at the top of the file
so the except block can call logging.warning without error (ensure logging is
imported before MAX_BATCH_SIZE is defined and used).
|
@smokeyScraper Hi there! I created issue #244 and have just uploaded my fix. I noticed a concurrent PR was opened, but I would ask you to consider my PR because it includes error-handling logic that prevents the server from failing to start due to configuration typos. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/app/database/falkor/code-graph-backend/api/graph.py`:
- Around line 53-61: Replace the broad Exception handler in _safe_create_index
with redis.exceptions.ResponseError: import ResponseError from redis.exceptions,
change the except Exception as e block to except ResponseError as e, and keep
the existing "already exists" string check and logging behavior (logging.info
for existing index, logging.error with exc_info for other ResponseError cases)
so unrelated exceptions (connection/timeouts/auth) are no longer swallowed by
this method.
| def _safe_create_index(self, func, label, *args): | ||
| try: | ||
| self.g.create_node_range_index("File", "name", "ext") | ||
| except Exception: | ||
| pass | ||
| func(label, *args) | ||
| logging.debug(f"Successfully created/verified index for '{label}'.") | ||
| except Exception as e: | ||
| if "already exists" in str(e).lower(): | ||
| logging.info(f"Index for '{label}' already exists.") | ||
| else: | ||
| logging.error(f"Failed to create index for '{label}': {e}", exc_info=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the full graph.py file to understand imports and context
find . -type f -name "graph.py" | grep -i falkorRepository: AOSSIE-Org/Devr.AI
Length of output: 123
🏁 Script executed:
# Look for requirements files to identify FalkorDB version
find . -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "Pipfile" \) | head -20Repository: AOSSIE-Org/Devr.AI
Length of output: 235
🏁 Script executed:
# Search for FalkorDB imports in the codebase
rg "falkor|FalkorDB" -i --type py | head -30Repository: AOSSIE-Org/Devr.AI
Length of output: 3693
🏁 Script executed:
# Check FalkorDB version in requirements
cat backend/app/database/falkor/code-graph-backend/requirements.txtRepository: AOSSIE-Org/Devr.AI
Length of output: 8289
🏁 Script executed:
# Also check the main requirements
cat backend/requirements.txt | grep -i falkorRepository: AOSSIE-Org/Devr.AI
Length of output: 44
🏁 Script executed:
# Read the full graph.py file to understand the context
wc -l backend/app/database/falkor/code-graph-backend/api/graph.pyRepository: AOSSIE-Org/Devr.AI
Length of output: 125
🏁 Script executed:
# Read the graph.py file
head -100 backend/app/database/falkor/code-graph-backend/api/graph.pyRepository: AOSSIE-Org/Devr.AI
Length of output: 3311
🏁 Script executed:
# Read lines around the _safe_create_index method (lines 53-61 plus surrounding context)
sed -n '1,100p' backend/app/database/falkor/code-graph-backend/api/graph.pyRepository: AOSSIE-Org/Devr.AI
Length of output: 3311
🏁 Script executed:
# Search for other exception handling patterns in the same file
rg "except" backend/app/database/falkor/code-graph-backend/api/graph.py -A 3Repository: AOSSIE-Org/Devr.AI
Length of output: 391
🏁 Script executed:
# Look for other FalkorDB exception handling in the codebase
rg "except.*falkor" -i backend/app/database/falkor/code-graph-backend/ -A 2Repository: AOSSIE-Org/Devr.AI
Length of output: 44
🌐 Web query:
FalkorDB 1.0.10 Python client exception types error handling
💡 Result:
In FalkorDB Python client v1.0.10, almost all runtime errors you’ll handle are redis-py exceptions, because the client is a thin wrapper around a redis.Redis(...) connection and ultimately calls execute_command(...) for FalkorDB commands. [1]
Exception types you should expect
1) FalkorDB-py custom exception
falkordb.exceptions.SchemaVersionMismatchException(the only documented custom exception type in the package). [2]
2) redis-py exceptions (primary ones to catch)
Catch from redis.exceptions (sync) / redis.asyncio (async), typically:
ResponseError: FalkorDB query/command errors (syntax errors, runtime errors like_AR_EXP_UpdateEntityIdx, etc.)ConnectionError: can’t connect / connection droppedTimeoutError: socket timeouts / blocked too longAuthenticationError: bad credentials- (optional umbrella)
RedisError: base class for most redis-py client exceptions
Practical error-handling pattern
from redis.exceptions import ResponseError, ConnectionError, TimeoutError, AuthenticationError, RedisError
from falkordb.exceptions import SchemaVersionMismatchException
try:
res = g.query("MATCH (n) RETURN n LIMIT 1")
except ResponseError as e:
# FalkorDB reported an error executing the command/query
msg = str(e)
# inspect msg for FalkorDB error codes/messages if you need branching
raise
except (ConnectionError, TimeoutError, AuthenticationError) as e:
# transport / auth problems
raise
except SchemaVersionMismatchException as e:
# schema cache/version mismatch (raised by the client’s schema machinery)
raise
except RedisError as e:
# any other redis-py error
raiseNotes:
- If you’re specifically seeing FalkorDB engine errors while running queries (common in
FalkorDB==1.0.10reports), they will surface asredis.exceptions.ResponseErrorbecause the server returns an error reply. [1] - v1.0.10 itself is a “version bump” release (plus connection-related tweaks like TLS support via
from_url), not an exceptions redesign—so the above model is what you should rely on. [3]
Sources:
[1] FalkorDB client builds on redis.Redis and exposes execute_command (readthedocs module source). (falkordb-py.readthedocs.io)
[2] SchemaVersionMismatchException definition (readthedocs exceptions module). (falkordb-py.readthedocs.io)
[3] v1.0.10 release notes / what changed. (github.com)
Citations:
- 1: https://falkordb-py.readthedocs.io/en/stable/_modules/falkordb/falkordb.html?utm_source=openai
- 2: https://falkordb-py.readthedocs.io/en/stable/_modules/falkordb/exceptions.html
- 3: https://github.com/FalkorDB/falkordb-py/releases/tag/v1.0.10
Catch redis.exceptions.ResponseError instead of broad Exception.
FalkorDB v1.0.10 is a thin wrapper around redis-py, so "index already exists" errors surface as redis.exceptions.ResponseError, not a FalkorDB-specific exception type. Catching Exception masks unrelated errors (connection failures, timeouts, authentication errors) that deserve different handling.
While string-based matching for "already exists" is not ideal, it remains the only viable approach since there's no specific error code exposed. However, catching ResponseError provides better specificity:
from redis.exceptions import ResponseError
def _safe_create_index(self, func, label, *args):
try:
func(label, *args)
logging.debug(f"Successfully created/verified index for '{label}'.")
except ResponseError as e:
if "already exists" in str(e).lower():
logging.info(f"Index for '{label}' already exists.")
else:
logging.error(f"Failed to create index for '{label}': {e}", exc_info=True)🤖 Prompt for AI Agents
In `@backend/app/database/falkor/code-graph-backend/api/graph.py` around lines 53
- 61, Replace the broad Exception handler in _safe_create_index with
redis.exceptions.ResponseError: import ResponseError from redis.exceptions,
change the except Exception as e block to except ResponseError as e, and keep
the existing "already exists" string check and logging behavior (logging.info
for existing index, logging.error with exc_info for other ResponseError cases)
so unrelated exceptions (connection/timeouts/auth) are no longer swallowed by
this method.
Closes #
The PR is : #244
📝 Description
This PR adds robust error handling for the
EMBEDDING_MAX_BATCH_SIZEenvironment variable. Previously, the application would encounter aValueErrorand crash during startup if the environment variable was set to a non-integer string.This change introduces a
try-exceptblock to ensure that if the environment variable is misconfigured, the system gracefully falls back to a default value of32and remains operational.🔧 Changes Made
EMBEDDING_MAX_BATCH_SIZEparsing in atry-exceptblock inbackend/config.py.📷 Screenshots or Visual Changes (if applicable)
N/A: This is a backend logic change for stability.
🤝 Collaboration
✅ Checklist
Summary by CodeRabbit